Ensure variables are set in case enlist_fixture_connections is called #42060
Conversation
called enlist_fixture_connections is currently called within the code when the context is `run_in_transasction?`. However if `enlist_fixture_connections` is called outside of that context the code fails since these variables are not defined. This ensures the code can still be executed without side effects
| @@ -111,12 +111,11 @@ def setup_fixtures(config = ActiveRecord::Base) | |||
| @fixture_connections = [] | |||
| @@already_loaded_fixtures ||= {} | |||
| @connection_subscriber = nil | |||
| @legacy_saved_pool_configs = Hash.new { |hash, key| hash[key] = {} } | |||
There was a problem hiding this comment.
While we're making changes in here can you condition these or make one instance var? I don't want the legacy one available when we're not in legacy mode.
There was a problem hiding this comment.
Done! I kept the variables separate but made sure they were only defined and used on their respective connection handling logic since it's more explicit and would allow us to just remove the legacy connection block once we remove that.
There was a problem hiding this comment.
@eileencodes I had to revert the change to condition these variables since it looks like the tests call out a setup and teardown with different values causing the variable to load values for legacy and then attempt to tear down non-legacy values and vice-versa.
This caused the failures seen here
Error:
--
BasePreventWritesTest::BasePreventWritesLegacyTest#test_preventing_writes_with_multiple_handlers:
--
| NoMethodError: undefined method `[]' for nil:NilClass
| /rails/activerecord/lib/active_record/test_fixtures.rb:209:in `block (2 levels) in setup_shared_connection_pool'
Using the same variable causes the same issue.
Tonkpils
force-pushed
the
tonkpils/fix-enlist-fixture-connections
branch
from
7195bcf
to
e088454
Compare
…onnections Ensure variables are set in case enlist_fixture_connections is called
Summary
#40384 Introduced a change which broke our tests at GitHub. We call
enlist_fixture_connectionswhen loading fixtures.The current Rails code uses
enlist_fixture_connectionswhen the context isrun_in_transasction?. However ifenlist_fixture_connectionsis called outside of that context the code fails since these variables are not defined. This ensures the code can still be executed without side effects.I wasn't sure how to or whether I should write a test for this since enlist_fixture_connections is a private method so feedback is appreciated.
Other Information
/cc @eugeneius as the original PR author